Skip to content

fix(rpc) Synchronize RPC requests only within the TP group. - #25213

Merged
xiezhq-hermann merged 3 commits into
sgl-project:mainfrom
twb1235:fix-rpc
Jul 18, 2026
Merged

xiezhq-hermann merged 3 commits into
sgl-project:mainfrom
twb1235:fix-rpc

Conversation

@twb1235

@twb1235 twb1235 commented May 14, 2026

Copy link
Copy Markdown
Contributor

Motivation

As discussed in issue #24687, there is a conflict between handle_rpc_request() and Pipeline Parallelism (PP) that causes a deadlock.

Root Cause:

When PP rank 0 receives an RPC request, the following sequence leads to a deadlock:

PP rank 0: recv_requests() → receives RPC request
PP rank 0: process_input_requests() → calls barrier() inside handle_rpc_request()
Other PP ranks: blocked in recv_requests() waiting for requests from rank 0 via point_to_point_pyobj
PP rank 0: hasn't sent requests yet (send happens after process) → DEADLOCK

Modifications

Solution:

Change barrier() in handle_rpc_request() to only synchronize within the TP group instead of globally:

Before

barrier() # Waits for all TP×PP processes

After

barrier(group=self.tp_group.device_group) # Only waits within TP group
This avoids the deadlock because:

The barrier no longer requires cross-stage synchronization (PP ranks don't need to wait for each other)
Each PP stage independently synchronizes its own TP ranks during RPC handling
Other PP ranks can continue executing recv_requests() without being blocked by rank 0's RPC barrier

Accuracy Tests

Speed Tests and Profiling

Checklist

Review and Merge Process

  1. Ping Merge Oncalls to start the process. See the PR Merge Process.
  2. Get approvals from CODEOWNERS and other reviewers.
  3. Trigger CI tests with comments or contact authorized users to do so.
    • Common commands include /tag-and-rerun-ci, /tag-run-ci-label, /rerun-failed-ci
  4. After green CI and required approvals, ask Merge Oncalls or people with Write permission to merge the PR.

CI States

Latest PR Test (Base): ⏳ Run #29631202458
Latest PR Test (Extra): ❌ Run #29631202342

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@twb1235

twb1235 commented May 14, 2026

Copy link
Copy Markdown
Contributor Author

/tag-run-ci-label

Comment thread python/sglang/srt/managers/scheduler.py Outdated
logger.error(f"Failed to call rpc {recv_req.method}: {str(e)}")

barrier()
barrier(group=self.tp_group.device_group)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why use device group? does tp_cpu_group work?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the reminder — yes, tp_cpu_group does work, and it's actually the more recommended approach.

@twb1235

twb1235 commented May 14, 2026

Copy link
Copy Markdown
Contributor Author

@merrymercy @Ying1123 @hnyls2002 @xiezhq-hermann Hi, could you please help review this PR?

@twb1235
twb1235 requested a review from ShangmingCai May 14, 2026 04:58

@ShangmingCai ShangmingCai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ShangmingCai

Copy link
Copy Markdown
Collaborator

/tag-and-rerun-ci

@ShangmingCai

Copy link
Copy Markdown
Collaborator

/rerun-failed-ci

1 similar comment
@twb1235

twb1235 commented May 16, 2026

Copy link
Copy Markdown
Contributor Author

/rerun-failed-ci

@twb1235

twb1235 commented May 18, 2026

Copy link
Copy Markdown
Contributor Author

@merrymercy @Ying1123 @hnyls2002 @xiezhq-hermann @cctry Hi, could you please help review this PR?

@twb1235

twb1235 commented Jul 16, 2026

Copy link
Copy Markdown
Contributor Author

Hi, sorry to bother you. This PR has been approved. When you have a moment, could you please help merge it? Thanks! @ShangmingCai @xiezhq-hermann

@xiezhq-hermann

Copy link
Copy Markdown
Collaborator

@copilot resolve the merge conflicts in this pull request

@xiezhq-hermann
xiezhq-hermann enabled auto-merge (squash) July 18, 2026 04:50
@xiezhq-hermann
xiezhq-hermann merged commit 071e649 into sgl-project:main Jul 18, 2026
130 of 159 checks passed
Zhylkaaa pushed a commit to Zhylkaaa/sglang that referenced this pull request Jul 29, 2026
jinzhenfan pushed a commit to jinzhenfan/sglang that referenced this pull request Jul 29, 2026
Chronostasys pushed a commit to MindLab-Research/sglang that referenced this pull request Aug 24, 2026
jakki-amd pushed a commit to jakki-amd/sglang that referenced this pull request Sep 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants